Add Stats Level config value for use with stats counter collection.#207
Add Stats Level config value for use with stats counter collection.#207
Conversation
zbyrne
requested review from
derobins,
gaoikawa,
jordan-turbofish,
kurtmcmillan and
riley-dixon
as code owners
|
It's early I realize and @derobins is re-working our docs, but we probably should add something to our public facing docs on stats collection 'soon'. hipFile -> docs -> (something related to stats, perhaps) Also, other libraries have a standalone environment variables explainer page, we should add something too. Okay to capture that in another follow-on story/PR. Thanks. |
There was a problem hiding this comment.
Pull request overview
This PR adds a HIPFILE_STATS_LEVEL environment variable and a corresponding statsLevel() configuration method to allow controlling the granularity of stats collection (for ais-stats). The feature mirrors the existing HIPFILE_FORCE_COMPAT_MODE pattern.
Changes:
- Adds
Environment::get<int>specialization andstats_level()convenience function in the environment layer. - Adds
statsLevel()to theIConfigurationinterface and the concreteConfigurationclass. - Adds unit tests for the new
get<int>environment parsing and thestatsLevel()configuration getter.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/amd_detail/environment.h |
Declares STATS_LEVEL constant and stats_level() method |
src/amd_detail/environment.cpp |
Implements get<int> template specialization and stats_level() |
src/amd_detail/configuration.h |
Adds statsLevel() pure virtual to IConfiguration and declaration to Configuration |
src/amd_detail/configuration.cpp |
Initializes m_statsLevel from environment and implements statsLevel() |
test/amd_detail/environment.cpp |
Unit tests for the new get<int> specialization |
test/amd_detail/configuration.cpp |
Unit tests for statsLevel() via ConfigurationExpectationBuilder |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zbyrne
force-pushed
the
zbyrne/stats-config
branch
from
bec9342 to
e2bd465
Compare
zbyrne
force-pushed
the
zbyrne/stats-config
branch
from
e2bd465 to
ecffff4
Compare
Motivation
AIHIPFILE-53 It's possible as we add more counters to ais-stats that we may start to see a performance overhead to the data collection. This setting will be used to let us control exactly how much data we collect from a given process in the event very detailed stats gathering has a large performance impact.
Technical Details
Adds an environment variable(HIPFILE_STATS_LEVEL) and Configuration method(::statsLevel()) in the same vein as HIPFILE_FORCE_COMPAT_MODE.
Test Plan
Added Environment and Configuration unit tests for the new functions.
Test Result
All unit tests pass.
Submission Checklist